-
Notifications
You must be signed in to change notification settings - Fork 4.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Disabled PCA inputs for deepTau v2 #27878
Disabled PCA inputs for deepTau v2 #27878
Conversation
The code-checks are being triggered in jenkins. |
-code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27878/11650
Code check has found code style and quality issues which could be resolved by applying following patch(s)
|
The code-checks are being triggered in jenkins. |
+code-checks Logs: https://cmssdt.cern.ch/SDT/code-checks/cms-sw-PR-27878/11652
|
A new Pull Request was created by @swozniewski for master. It involves the following packages: RecoTauTag/RecoTau @perrotta, @cmsbuild, @slava77 can you please review it and eventually sign? Thanks. cms-bot commands are listed here |
please test |
The tests are being triggered in jenkins. |
@swozniewski @mbluj, in view of the possible discussion at the ORP, could you please let us know which are the plans for this PR and its backports? I.e. are they planned for a given re-miniAOD campaign, or which is the urgency for having them ready for analyses? Moreover, do you have updated plots as in page 2 of https://indico.cern.ch/event/842796/contributions/3541592/attachments/1897386/3130770/2019-08-26_DeepTau_ID_2017v2_DY_MC_vs_Embedded_and_bug_in_definition_of_PCA_variables.pdf made with the updated id as in this PR? |
Let me comment here, yes, we will want this backported to 10_2 and 10_6 because it's the version that should replace v2 for Run2 data analysis. And yes, it will be included in a future MINIAOD run. We can discuss at the ORP how to handle this (I guess with the usual run2_miniAOD_devel), probably we will want to switch it on before UL18 starts. |
Comparison job queued. |
Backports of this PR are already prepared:
To redo those plots a number of samples has to be processed. Such processing with 102X is well advanced, plots can be expected at the end of this week or beginning of next one. |
Comparison is ready Comparison Summary:
|
+1
|
+1 |
+1 |
This pull request is fully signed and it will be integrated in one of the next master IBs (tests are also fine). This pull request will now be reviewed by the release team before it's merged. @davidlange6, @slava77, @smuzaffar, @fabiocos (and backports should be raised in the release meeting by the corresponding L2) |
+1 |
For records: a set of control plots obtained with the fixed DeepTau 2017v2p1, i.e. with PCA inputs disabled, can be found in https://indico.cern.ch/event/847915/contributions/3565600/attachments/1908653/3153065/TauPOG_DeepTauIDv2p1.pdf (for instance see sl. 4-5 and 12-14). The fix improves data/MC agreement as expected for whole Run-2, in particular ~30% disagreement for 2017 is solved. |
PR description:
Problems have been detected with dxy_PCA input variables to DeepTauID which are inconsistent between data and MC. This PR sets input values to 0 and disables the variables this way.
Issue and fix were discussed in https://indico.cern.ch/event/842796/contributions/3541592/attachments/1897386/3130770/2019-08-26_DeepTau_ID_2017v2_DY_MC_vs_Embedded_and_bug_in_definition_of_PCA_variables.pdf (see slide 7 in particular)
Backport to 10_2_X for legacy analyses intended.
PR validation:
compiles